-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecation on implicit bool conversions to true for values other than 1 #8565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Zend/zend_operators.c
Outdated
@@ -836,6 +836,19 @@ ZEND_API void ZEND_COLD zend_incompatible_string_to_long_error(const zend_string | |||
zend_error(E_DEPRECATED, "Implicit conversion from float-string \"%s\" to int loses precision", ZSTR_VAL(s)); | |||
} | |||
|
|||
ZEND_API void ZEND_COLD zend_incompatible_double_to_bool_error(double d) | |||
{ | |||
zend_error_unchecked(E_DEPRECATED, "Implicit conversion from float %.*H to true, only 0 or 1 are allowed", -1, d); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry if I missed something but why not "0.0 or 1.0"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be an option too (and that was the initial message). I changed it to say 0 or 1 for both float and int to steer a developer towards thinking about the value and not the type, as explicitely using 0.0 or 1.0 as a boolean argument seems like dodgy advice.
I fixed the failing iterator tests so they are not affected by this deprecation, and I created the RFC which contains more details: https://wiki.php.net/rfc/stricter_implicit_boolean_coercions |
A deprecation notice appeared on Travis in |
I'm expecting OpCache to need to have changes made as it now needs to bail out off certain optimizations. |
Co-authored-by: George Peter Banyard <girgias@php.net>
I started to look at Opcache and if I need to change anything there, but nothing stuck out so far (but remember, I am super new to the php-src codebase). Are there tests for Opcache/JIT for something like this, or would it make sense to add more tests specifically for something like this to opcache/jit? I managed to fix all other existing tests with the exception of the AppVeyor build: there are just two failures left, one failure is in |
Compile with |
Thanks! I did both and all the tests passed. |
RFC has been declined |
This is a preliminary implementation of an upcoming RFC about Stricter implicit boolean coercions (see the previous discussion on https://externals.io/message/117608).
This is my first attempt at a PHP contribution, which is why I tried to mimick what the implicit float-to-int deprecation implementation did (#6661). I added some additional tests and adapted the existing tests where deprecation notices appeared.
EDIT: Only two tests are still failing in the AppVeyor build (to do with large ints appearing as
-1
- see a comment further below for details). If anything else needs to be changed or can be improved I welcome any feedback. The RFC can be found on https://wiki.php.net/rfc/stricter_implicit_boolean_coercions